Skip to content

Python: Make "Modification of parameter with default" flow-sensitive.#878

Merged
markshannon merged 4 commits into
github:masterfrom
tausbn:python-mutable-default-with-flow
Mar 1, 2019
Merged

Python: Make "Modification of parameter with default" flow-sensitive.#878
markshannon merged 4 commits into
github:masterfrom
tausbn:python-mutable-default-with-flow

Conversation

@taus-semmle

@taus-semmle taus-semmle commented Feb 4, 2019

Copy link
Copy Markdown
Contributor

Extends the analysis in py/modification-of-default-value to use taint tracking. This enables us to detect modifications that happen inside other function calls.

@felicity-semmle for the change note. (And also possibly the alert message?)

Open question:

I have encountered the following idiom in some places that are flagged by the current query

def foo(x=[]):
    if not x:
        x = [1]
    # ... more statements, none of which modify x

Should we treat if not x as a sanitiser for x? As far as I can tell, there are no hidden surprises in the above code.

@felicitymay felicitymay left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for pinging me. One minor suggestion, otherwise the text lgtm.

Comment thread change-notes/1.20/analysis-python.md Outdated
@ghost

ghost commented Feb 5, 2019

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@felicitymay

Copy link
Copy Markdown
Contributor

Thanks for the change. Text LGTM.

@markshannon markshannon left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. We can add handling of taint truthiness for another PR.

@markshannon markshannon merged commit 8a16164 into github:master Mar 1, 2019
@tausbn tausbn deleted the python-mutable-default-with-flow branch February 12, 2021 18:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants